AP-870: Make location requests OCLC limit configurable - #61
Conversation
Read FRAMEWORK_LOCATION_REQUESTS_MAX_OCLC_NUMBERS through altmedia configuration and expose it on Rails configuration so LocationRequest::MAX_OCLC_NUMBERS can be adjusted without a code change. Add a sample env value and targeted coverage for config parsing, display formatting, and the over-limit spreadsheet check. Co-authored-by: Codex GPT-5 <noreply@openai.com>
davezuckerman
left a comment
davezuckerman
left a comment
There was a problem hiding this comment.
Just had a question about the defaults being 50000. Also I'm assuming there will be another pull request to update the wording on the view which says to limit requests to 50000.
|
@davezuckerman the form partial already calls |
awilfox
left a comment
awilfox
left a comment
There was a problem hiding this comment.
r+wc after the default is fixed.
davezuckerman
left a comment
davezuckerman
left a comment
There was a problem hiding this comment.
Looks good after the test is updated for 10000 as opposed to 50000
awilfox
left a comment
awilfox
left a comment
There was a problem hiding this comment.
r+wc with the logic of my other review swapped.
Co-authored-by: Anna Wilcox <AWilcox@Wilcox-Tech.com>
|
The test failures are in the location request spec; since it's testing the handling of large requests, we may want to temporarily inject a higher cap into the environment for those tests. Otherwise this looks good. |
Update the configuration spec for the new 10,000 default and keep the large-record LocationRequest model spec above the batch boundary without depending on the default cap. Co-authored-by: Codex GPT-5 <noreply@openai.com>
3 participants
Read
FRAMEWORK_LOCATION_REQUESTS_MAX_OCLC_NUMBERSthrough altmedia configuration and expose it on Rails configuration soLocationRequest::MAX_OCLC_NUMBERScan be adjusted without a code change.Add a sample env value and targeted coverage for config parsing, display formatting, and the over-limit spreadsheet check.